Add the rite_calendar_test object type to the LiturgicalCalendar model - #31
Conversation
LiturgicalCalendarAPI#785 introduced a new FGA object type, `rite_calendar_test`, and started querying it from `ResourceAdminService::VIEWER_OBJECT_TYPES` / `TEST_OBJECT_TYPES`. It generalises `general_roman_calendar_test`, whose single fixed id (`general_roman_calendar`) could only ever name the Roman rite-level calendar; the new type's object id is the bare rite, so `rite_calendar_test:roman` succeeds it and `rite_calendar_test:ambrosian` is the scope the generalisation exists for. The type was never added here. #785 edited `LiturgicalCalendarAPI/scripts/openfga-model.additive.json`, a file that repo's own runbook already declares dead — model changes go through a PR in this repo, followed by an operator upload on the VPS. So the type exists in the API's allow-lists and in nothing that is deployed. Relations are `admin` / `editor` / `viewer`, identical to `general_roman_calendar_test`: `editor` is a union including `admin`, `viewer` a union including both, each directly relatable to `user`. That is what the API actually queries (listObjects for `viewer`, `editor`, and `admin`; the authorization middleware checks `admin` for PUT/DELETE and `editor` for PATCH), and it is what `LiturgicalCalendarAPI/docs/ops/test-scope-migration-runbook.md` Step 1 specifies. No `deleter`: the consumer's own expectations file forbids that relation model-wide, and the dead file's copy of the type carries one — one more reason not to have mirrored it. Purely additive. `general_roman_calendar_test` is untouched, because pre-migration tuples on it must keep authorizing until `scripts/migrate-rite-test-tuples.php --apply --prune` has run everywhere. Verified against the consumer contract rather than by inspection: with the registry entry from #30, `validate-expectations.sh` reports `required_types: type "rite_calendar_test" not found in model` (exit 1) against main's model, and `satisfied` (exit 0) against this one. `LiturgicalCalendar.lock.json` is deliberately not touched — the model ID is whatever the store returns on upload, and setup-openfga.sh reports the lock JSON for a human to commit afterwards. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (2)
Included review availability: Your plan includes up to 1 review per rolling hour; 0 remain after this review. 📝 WalkthroughWalkthroughThe authorization model adds the ChangesLiturgical calendar authorization
Estimated code review effort: 2 (Simple) | ~10 minutes Merge Risk: ⚪ Minimal · up to This PR adds the missing calendar object type, and no actionable merge-blocking risk remains; it is merge-ready after normal checks and review. 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
What broke
LiturgicalCalendarAPI#785 introduced a new OpenFGA object type,rite_calendar_test, and started querying it fromResourceAdminService::VIEWER_OBJECT_TYPESandTEST_OBJECT_TYPES. It generalisesgeneral_roman_calendar_test, whose single fixed id (general_roman_calendar) could only ever name the Roman rite-level calendar; the new type's object id is the bare rite, sorite_calendar_test:romansucceeds it andrite_calendar_test:ambrosianis the scope the generalisation exists for.The type was never added to the deployed model. #785 edited
LiturgicalCalendarAPI/scripts/openfga-model.additive.json— a file that repo's own runbook (docs/ops/rbac-create-governance-runbook.md, lines 16-19) already declares dead:No such PR was ever opened. This is it. (That dead file is being deleted in a companion PR on the API side, since looking like a live surface is exactly how this happened.)
Observable symptom — global, not scoped
listObjectson an unknown type returns400 type_not_found, which the API'sOpenFgaClientsurfaces as aRuntimeException. InResourceAdminServicetheforeachover object types sits inside a singletry, and thecatch (\RuntimeException)returnsarray_fill_keys(VIEWER_OBJECT_TYPES, [])— so one unknown type empties every type's scope list, for every user. Dashboard card gating therefore collapses globally, not just for rite-scoped tests.Confirmed against a dev store: its latest model contains
user,wider_region,national_calendar,diocesan_calendar,general_roman_calendar,national_calendar_test,diocesan_calendar_test,general_roman_calendar_test— and norite_calendar_test.Downstream
LiturgicalCalendarFrontende2e specs this unblocks:rbac/07-dashboard-card-scoping:143rbac/12rbac/13:153andrbac/13:201rbac/15-dashboard-tests-card-matrix:36The change
rite_calendar_testwith relationsadmin/editor/viewer, identical togeneral_roman_calendar_test:editoris a union includingadmin,viewera union including both, each directly relatable touser.That relation set is not a guess. It is:
listObjectsforviewer,editorandadmin(ResourceAdminService), andOpenFgaAuthorizationMiddlewarecheckingadminforPUT/DELETEandeditorforPATCH;LiturgicalCalendarAPI/docs/ops/test-scope-migration-runbook.mdStep 1 specifies — "added […] alongsidegeneral_roman_calendar_test, with an identical relation set";national_calendar_test/diocesan_calendar_testtypes already in this file.No
deleter. The consumer's own expectations file forbids that relation model-wide ("forbidden_relations": {"*": ["deleter"]}), and the dead file's copy of the type carries one — one more reason not to have mirrored it verbatim.Purely additive.
general_roman_calendar_testis untouched: pre-migration tuples on it must keep authorizing untilscripts/migrate-rite-test-tuples.php --apply --prunehas run in every environment, and the API still accepts the old type everywhere.auth/handoffs/liturgicalcalendar.mdgets a note rather than an edited type list — the list there describes deployed model01KW4FW2ZCT1E693PY8D9TJEFM, which genuinely does not contain this type yet.Verification
Not by inspection — against the consumer contract itself, using the registry entry from #30:
./auth/validate-expectations.selftest.sh— all 21 cases behave as declared.Coordination with #30
#30 should not merge before this PR.
auth/models/consumers.jsononmainis still[], sovalidate-modelson this PR passes trivially (empty registry = nothing to contradict) — its green tells you nothing, hence the manual run above. #30 registersLiturgicalCalendarAPIagainstdevelopment's expectations file, which already listsrite_calendar_testinrequired_types. #30's own CI ran on 2026-08-05, before that line existed; re-run today it goes red, and merging it first turnsmainred on the push-triggered run. Landing this first makes #30 green again with no change to #30 itself.Deploy step required — this PR alone changes nothing live
Merging only updates the model file. An operator must then, per
auth/README.md:cd /opt/cdcf-auth/auth ./setup-openfga.sh --target production --create-litcal-storeThe lock guard will refuse (exit 7) if the store's latest model does not match
LiturgicalCalendar.lock.json; otherwise it uploads and prints the new model ID plus the lock JSON to commit. That lock update is a follow-up PR — this one deliberately leavesLiturgicalCalendar.lock.jsonalone, since the script never writes it and the ID is not knowable before the upload. Any consumer pinningOPENFGA_MODEL_IDneeds the new ID too, and theliturgicalcalendar.mdhandoff note should be resolved at the same time.Local dev stacks (including the frontend e2e store) pick the type up by re-running
setup-openfga.sh --target localagainst a clone of this repo after merge — the e2e specs above stay red until that happens.🤖 Generated with Claude Code
Summary by CodeRabbit
New Features
Documentation